-
Notifications
You must be signed in to change notification settings - Fork 184
fix migration with bridge interface be more stable #6780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix migration with bridge interface be more stable #6780
Conversation
WalkthroughThis test file modification adds an import of the Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py`:
- Around line 211-213: The SSH prompt is hardcoded as '#' in the remote_session
creation (aexpect.remote.remote_login) which breaks for non-root users or custom
prompts; update the prompt argument to use a regex pattern such as r'[$#%]' (or
the same configurable prompt pattern used across other migration tests) so the
login matches common shell prompts and avoids hangs/timeouts.
🧹 Nitpick comments (1)
libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py (1)
175-179: Make the teardown delay configurable or readiness-based.
A fixed 5s sleep can still be flaky (too short) or wasteful (too long). Consider a parameterized wait or polling for connectivity.♻️ Proposed tweak (parameterize the wait)
- # Add some waiting time to make host connection stable after clean bridge. - time.sleep(5) + # Add some waiting time to make host connection stable after clean bridge. + bridge_cleanup_wait = int(params.get("bridge_cleanup_wait", "5")) + time.sleep(bridge_cleanup_wait)
| remote_session = aexpect.remote.remote_login("ssh", server_ip, "22", | ||
| server_user, server_pwd, | ||
| r'[$#%]') | ||
| '#') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check if the file exists and examine the relevant lines
if [ -f "libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py" ]; then
echo "=== File found. Checking lines 205-220 for context ==="
sed -n '205,220p' libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py
else
echo "File not found at expected path. Searching for it..."
find . -name "migrate_with_bridge_type_interface.py" -type f 2>/dev/null
fiRepository: autotest/tp-libvirt
Length of output: 653
🏁 Script executed:
#!/bin/bash
# Check existing prompt usage patterns across the repo
echo "=== Remote login prompt patterns ==="
rg "remote_login" -A 3 -B 1 --max-count 20Repository: autotest/tp-libvirt
Length of output: 25885
🏁 Script executed:
#!/bin/bash
# Check for params.get usage patterns in test files
echo "=== Checking params.get patterns in test files ==="
rg "params\.get.*prompt" --type py -C 2Repository: autotest/tp-libvirt
Length of output: 10806
Fix SSH prompt hardcoding to support non-root users and custom prompts.
The literal '#' prompt fails for non-root users and systems with custom shell prompts, causing login hangs/timeouts. This is inconsistent with other migration test files in the same codebase, which use configurable patterns like r'[$#%]'.
Fix: Use regex pattern matching (lines 211-213)
remote_session = aexpect.remote.remote_login("ssh", server_ip, "22",
- server_user, server_pwd,
- '#')
+ server_user, server_pwd,
+ r'[$#%]')Alternatively, for test parameterization:
+ remote_prompt = params.get("remote_prompt", r'[$#%]')
remote_session = aexpect.remote.remote_login("ssh", server_ip, "22",
- server_user, server_pwd,
- '#')
+ server_user, server_pwd,
+ remote_prompt)🤖 Prompt for AI Agents
In
`@libvirt/tests/src/virtual_network/migrate/migrate_with_bridge_type_interface.py`
around lines 211 - 213, The SSH prompt is hardcoded as '#' in the remote_session
creation (aexpect.remote.remote_login) which breaks for non-root users or custom
prompts; update the prompt argument to use a regex pattern such as r'[$#%]' (or
the same configurable prompt pattern used across other migration tests) so the
login matches common shell prompts and avoids hangs/timeouts.
fd5910d to
ed8ed0f
Compare
no sleep after case running would make bridge clean and host get ip process be more stable Signed-off-by: nanli <[email protected]>
ed8ed0f to
3f4aa04
Compare
|
No need to update this pr, All fixes are in avocado-framework/avocado-vt#4309 |
no sleep after case running would make bridge clean and host get ip process be more stable
Signed-off-by: nanli [email protected]
Also need avocado-framework/avocado-vt#4309 to make case pass
Before fixed:
virtual_network.migrate.migrate_with_bridge_type_interface.start_with_interface.precopy_migration.ovs_bridge FATAL: command execution failed
After fixed:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.